Use input as data in eth_sendTransaction #300
Conversation
src/wallet.ts
Outdated
| const txParamsData = data || input; | ||
| const txParams: TransactionParams & { data?: string } = { | ||
| ...restParams, | ||
| ...(txParamsData ? { data: txParamsData } : undefined), |
There was a problem hiding this comment.
For initial maximal compatibility, how about setting both input and data, removing one in a follow-up release?
After this change, input is now a special case filtered out from output while any other free-name fields are still passed through. This is a bit counter-intuitive and might be good to do in a two-step process?
| ...(txParamsData ? { data: txParamsData } : undefined), | |
| ...( | |
| txParamsData | |
| ? { data: txParamsData, input: txParamsData } | |
| : undefined | |
| ), |
There was a problem hiding this comment.
are we sure we want to set both? the original geth PR says:
bail if both are set
There was a problem hiding this comment.
Should we really modify the params at all? Considering there are other implementations which may work differently. Maybe the validation that both aren't provided with differing values is enough for the sake of this package?
There was a problem hiding this comment.
The only consumer of this should be TransactionController, which drops input (and other unexpected fields) as one of the first steps of transaction processing
There was a problem hiding this comment.
This is a public package with external dependents who rely on the current API, though, which also need to be considered.
https://www.npmjs.com/browse/depended/@metamask/eth-json-rpc-middleware
There are likely other dependents as well, which don't necessarily show up on npmjs.
There was a problem hiding this comment.
MetaMask is also actively advising existing users of deprecated web3-provider-engine to migrate to @metamask/eth-json-rpc-middleware.
The Ethereum execution API specs have
eth_sendTransactionwith ainputfield, however nodes such as geth have been using thedatafield instead, addinginputsupport later.To make
eth_sendTransactionalign better with execution api, this PR makeseth_sendTransactionacceptsinputORdata(if both are provided, they must be equal) and maps it to thedatafield in the object that theprocessTransactioncallback expectsSee: https://github.com/MetaMask/MetaMask-planning/issues/2214